Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"IsDuring" function #110

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

"IsDuring" function #110

wants to merge 9 commits into from

Conversation

arran4
Copy link
Owner

@arran4 arran4 commented Oct 15, 2024

Pending merge with #99 for resolution

Fixes: #70

As requested.

This is broken and will need to be manually merged with:

#107

In order to fix the outstanding issues with it.

Please review Mereep @Mereep And let me know if it is what you had in mind, there are some TODO I added as I need your input on in order to complete this, this isn't functionality I have used.

Someone could also add aggregate / filtering functions to this to help people get "all the matching events" etc.

@arran4 arran4 self-assigned this Oct 15, 2024
This was referenced Oct 15, 2024
* origin/master:
  Duplication issue fixed.
  Some multiple-ness.
  Test fixed.
  Resynced and moved some functions around #78
  added functionnal option pattern for url parsing
  usage example, README
  calendar parsing url support

# Conflicts:
#	calendar.go
@arran4
Copy link
Owner Author

arran4 commented Oct 16, 2024

Conflict resolved.

* master:
  Merge remote-tracking branch 'origin/master' into issue97
  Fix
  Some larger upgrades.
  Tests added.

# Conflicts:
#	property_test.go
@arran4 arran4 added the blocked label Oct 17, 2024
@arran4
Copy link
Owner Author

arran4 commented Oct 28, 2024

@brackendawson Sorry for tagging you in a new (to you) PR.

While this is originally for checking if some date is in between a date. -- it sprawled out a bit -- I found a major issue with the use of .UTC() that relates heavily to #107

I think I will have to do a major version bump or something, there is a lot I'm not entirely content with having dug into timezones.

I haven't read your comments yet on #107 I've run out of time today. But I should get some time on Wednesday AEST but a quick glance slightly suggests to me that they might be related. Let me know your thoughts

@brackendawson
Copy link
Contributor

brackendawson commented Oct 28, 2024

If my proposal were adopted for #99/#107, having a configurable fallback time-zone scoped to the Calendar instance, then this would be simple, something similar to:

// Contains returns true if the given time falls within the event. Within is
// defined as equal to or after the start and before the end.
func (c *ComponentBase) Contains(t time.Time) (bool, error) {
    end, err := e.GetStartAt()
    if err != nil {
        if errors.Is(err, ErrorPropertyNotFound) {
            return false, nil
        }
        return false, err
    }
    if t.Before(start) {
        return false, nil
    }

    end, err := e.GetEndAt()
    if err != nil {
        if errors.Is(err, ErrorPropertyNotFound) {
            return false, nil
        }
        return false, err
    }
    if t.After(end) || t.Equal(end) {
        return false, nil
    }

    return true, nil
}

Go will handle the time zones for you.

@arran4
Copy link
Owner Author

arran4 commented Oct 28, 2024

I will have to produce some sort of VTIMEZONE map to go timezone at some point. However if it's anything like timezones go doesn't support the concept of a floating (RFC's term) time as far as I know as it's heart is a ns unix-time-like implementation:

https://github.com/golang/go/blob/master/src/time/time.go#L140-L161

I just pray the timezone information is isomorphic when I get around to that.

In terms of the other components of the PR opinion?

@arran4
Copy link
Owner Author

arran4 commented Oct 29, 2024

The closest the spec goes to "global timezone" is:

https://www.rfc-editor.org/rfc/rfc5545

[3.8.3.1](https://www.rfc-editor.org/rfc/rfc5545#section-3.8.3.1).  Time Zone Identifier

   Property Name:  TZID

   Purpose:  This property specifies the text value that uniquely
      identifies the "VTIMEZONE" calendar component in the scope of an
      iCalendar object.

   Value Type:  TEXT

   Property Parameters:  IANA and non-standard property parameters can
      be specified on this property.

   Conformance:  This property MUST be specified in a "VTIMEZONE"
      calendar component.

   Description:  This is the label by which a time zone calendar
      component is referenced by any iCalendar properties whose value
      type is either DATE-TIME or TIME and not intended to specify a UTC
      or a "floating" time.  The presence of the SOLIDUS character as a
      prefix, indicates that this "TZID" represents an unique ID in a
      globally defined time zone registry (when such registry is
      defined).

         Note: This document does not define a naming convention for
         time zone identifiers.  Implementers may want to use the naming
         conventions defined in existing time zone specifications such
         as the public-domain TZ database [[TZDB](https://www.rfc-editor.org/rfc/rfc5545#ref-TZDB)].  The specification of
         globally unique time zone identifiers is not addressed by this
         document and is left for future study.

   Format Definition:  This property is defined by the following
      notation:

       tzid       = "TZID" tzidpropparam ":" [tzidprefix] text CRLF

       tzidpropparam      = *(";" other-param)

       ;tzidprefix        = "/"
       ; Defined previously. Just listed here for reader convenience.

   Example:  The following are examples of non-globally unique time zone
      identifiers:

       TZID:America/New_York

       TZID:America/Los_Angeles

      The following is an example of a fictitious globally unique time
      zone identifier:

       TZID:/example.org/America/New_York

Which is per component. Also note, TZ can be custom specified. Which means I need to parse and understand RRULE. But I'm not sure how to convert that to time.Location as it's all private: https://github.com/golang/go/blob/master/src/time/zoneinfo.go

Heh amusing:
https://github.com/golang/go/blob/master/src/time/zoneinfo.go#L640-L641

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IsInEvent functionality
2 participants